Python: MaD barriers #21004
Open
Python: MaD barriers #21004
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll
Fixed
Show fixed
Hide fixed
yoff
force-pushed
the
python/mad-barriers
branch
from
7d180be to
de4d4a6
Compare
yoff
force-pushed
the
python/mad-barriers
branch
from
e8d497b to
0a6a7ec
Compare
yoff
force-pushed
the
python/mad-barriers
branch
from
bb141ab to
2e78b14
Compare
and reinstate previously removed barrier now as a MaD row
- consider if the model is in the right place - consider if the barrier kind (sink kind) is the appropriate one
yoff
force-pushed
the
python/mad-barriers
branch
from
f89dd10 to
d5e792d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds Models-as-Data (MaD) support for barriers and barrier guards across Python, Ruby, and JavaScript, along with convenience predicates to simplify their usage.
Changes:
- Introduces
barrierModelandbarrierGuardModelextensible predicates in all three languages - Adds convenience predicates
sourceNode,sinkNode, andbarrierNodeto replace verbose patterns likegetASourceNode(kind).asSource() - Implements
ExternalBarrierGuardandParameterizedBarrierGuardmodules for handling barriers defined via MaD - Migrates several existing barriers to the new MaD framework (e.g., Django's
url_has_allowed_host_and_scheme, Ruby'sRegexp.escape/quote, JavaScript'sencodeURIComponent/encodeURI)
Reviewed changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| python/ruby/javascript: ApiGraphModelsExtensions.qll | Adds extensible predicates for barrier and barrier guard models |
| python/ruby/javascript: ApiGraphModels.qll | Implements internal support for barrier models and adds convenience predicates (sourceNode, sinkNode, barrierNode) |
| python/ruby/javascript: empty.model.yml | Registers new extensible predicates in data extension system |
| python: DataFlowPublic.qll | Adds ParameterizedBarrierGuard and ExternalBarrierGuard modules |
| ruby: DataFlowPublic.qll | Adds ParameterizedBarrierGuard and ExternalBarrierGuard modules |
| ruby: SsaImpl.qll | Adds ParameterizedBarrierGuard module at SSA level |
| javascript: BarrierGuards.qll | Adds ExternalBarrierGuard module |
| python/ruby/javascript: Multiple customization files | Migrates from verbose .getASinkNode(kind).asSink() to concise sinkNode(node, kind) pattern |
| ruby: regexp/model.yml | Defines Regexp.escape/quote as barriers for regexp-injection |
| python: Django.model.yml | Defines url_has_allowed_host_and_scheme as barrier guard for url-redirection |
| javascript: NodeJSLib.model.yml | Defines encodeURIComponent/encodeURI as barriers for request-forgery |
| python: Django.qll | Removes hand-coded Django URL validation barrier in favor of MaD model |
| ruby: RegExpInjectionCustomizations.qll | Removes hand-coded Regexp.escape barrier in favor of MaD model |
| javascript: IncompleteHtmlAttributeSanitizationCustomizations.qll | Replaces EncodingSanitizer with MaD-based sanitizer |
| python: UrlRedirectCustomizations.qll | Adds SanitizerFromModel for MaD-defined barriers |
| Test expectation files | Updates MaD IDs due to new extensible predicates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this = DataFlow::globalVarRef(["encodeURIComponent", "encodeURI"]).getACall() | ||
| } | ||
| private class SanitizerFromModel extends Sanitizer { | ||
| SanitizerFromModel() { ModelOutput::barrierNode(this, "request-forgery") } |
There was a problem hiding this comment.
The barrier kind "request-forgery" appears to be incorrect for this context. This class is in the IncompleteHtmlAttributeSanitization module, where encodeURIComponent/encodeURI were being used as sanitizers for incomplete HTML attribute sanitization vulnerabilities. The barrier kind should likely be something related to HTML injection or incomplete HTML attribute sanitization, not "request-forgery".
Suggested change
| SanitizerFromModel() { ModelOutput::barrierNode(this, "request-forgery") } | |
| SanitizerFromModel() { ModelOutput::barrierNode(this, "incomplete-html-attribute-sanitization") } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The example shows how to build a barrier guard from a MaD row. Clearly, this should be done in a convenience predicate.